Thanks, the failure on Travis is unrelated.

Could you add a note to whatsnew/v0.21.1.txt and a unit test (maybe using the example from #18044)?

Hello @ogotaiking! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on November 07, 2017 at 13:51 Hours UTC

Codecov Report

Merging #18065 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #18065      +/-   ##
==========================================
- Coverage   91.25%   91.23%   -0.02%     
==========================================
  Files         163      163              
  Lines       50120    50120              
==========================================
- Hits        45737    45728       -9     
- Misses       4383     4392       +9

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15fa4bd...58299cb. Read the comment docs.

Codecov Report

Merging #18065 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #18065      +/-   ##
==========================================
- Coverage   91.25%   91.23%   -0.02%     
==========================================
  Files         163      163              
  Lines       50124    50124              
==========================================
- Hits        45742    45733       -9     
- Misses       4382     4391       +9

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc69dc6...ee5a33d. Read the comment docs.

pls rebase on master as well.

@jreback for the comments for return Nan for numerically unstable issue[ seems lost the comments during rebase]

consider the uniform distribution {Xi}, X1=X2=....Xn-1=Xn, the variance sum(Xi-Xm)^2/n should be Zero, but some floating issue cause the variance is in range(0,1e-14), and the variance is in the denominator, that will cause skew/kurt result with an extremely large number. so we can treat if the variance is less than 1e-14, it's uniform distribution.

The original code only checks if the variance == Zero, and return Nan for uniform distribution, so I don't want to break this behavior and leave the result as Nan.

some reference :
http://www.wolframalpha.com/input/?source=nav&i=skewness%7B1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1%7D

http://www.wolframalpha.com/input/?source=nav&i=kurtosis%7B1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1,1.1%7D

result is Nan(undefined)

@ogotaiking not sure of your last, is this a different case?

@jreback same case. and also a reference from original skewness calculation code

pandas/core/nanops.py

def nanskew(values, axis=None, skipna=True):
    ...
    # floating point error
    m2 = _zero_out_fperr(m2) <- here m2 is eq B in rolling_skew
    m3 = _zero_out_fperr(m3)

    with np.errstate(invalid='ignore', divide='ignore'):
        result = (count * (count - 1) ** 0.5 / (count - 2)) * (m3 / m2 ** 1.5)

and the function _zero_out_fperr

_zero_out_fperr(arg):
    if isinstance(arg, np.ndarray):
        with np.errstate(invalid='ignore'):
            return np.where(np.abs(arg) < 1e-14, 0, arg)
    else:
        return arg.dtype.type(0) if np.abs(arg) < 1e-14 else arg

so here we treat the B <1e-14 is eq 0 in calc_skew and calc_kurt is following the original skew/kurt behavior.

@ogotaiking ok I c. can you add update the comment in the code above to include a refernce to _zero_fperr (and maybe a reference in zero_fperr to calc_kurt/calc_skew), just so that future readers will know that these are connected. otherwise lgtm. ping when pushed / green.

lgtm merge on green

thanks @ogotaiking